Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): add multimodal support #996

Merged
merged 44 commits into from
Nov 16, 2024
Merged

feat(core): add multimodal support #996

merged 44 commits into from
Nov 16, 2024

Conversation

hassiebp
Copy link
Contributor

@hassiebp hassiebp commented Nov 12, 2024

Important

This PR adds multimodal support with enhanced media management, task management updates, and API client improvements for media operations.

  • Multimodal Support:
    • Add MediaManager in media_manager.py for handling multimodal events and media uploads.
    • Update IngestionConsumer in ingestion_consumer.py to process multimodal events using MediaManager.
    • Introduce MediaUploadQueue and MediaUploadConsumer for managing media uploads.
  • Task Management:
    • Rename Consumer to IngestionConsumer and update its logic for multimodal data.
    • Refactor TaskManager to support separate media uploads and ingestion.
  • API Client:
    • Add media-related endpoints and models in client.py and resources/media.
    • Update README.md to reflect API usage changes.
  • Testing:
    • Add tests for multimodal support in test_langchain.py and test_task_manager.py.
    • Update existing tests for task management and ingestion changes.

This description was created by Ellipsis for d19f4f1. It will automatically update as commits are pushed.

@hassiebp hassiebp marked this pull request as draft November 12, 2024 13:40
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disclaimer: Experimental PR review

PR Summary

Added multimodal support to the Python SDK with new task management system for handling media files and comments.

  • Added MediaManager in /langfuse/_task_manager/media_manager.py for processing and uploading images/audio with base64 data URI parsing and SHA256 validation
  • Introduced async media upload queue system with MediaUploadQueue and MediaUploadConsumer for reliable background processing
  • Added new API clients and types for media operations (/langfuse/api/resources/media/) with presigned URL support and proper error handling
  • Added comments functionality with new API clients and types (/langfuse/api/resources/comments/) for attaching comments to traces, observations, sessions and prompts
  • Refactored task manager to separate ingestion and media upload concerns with proper thread safety and resource management

48 file(s) reviewed, 25 comment(s)
Edit PR Review Bot Settings | Greptile

langfuse/_task_manager/ingestion_consumer.py Outdated Show resolved Hide resolved
langfuse/_task_manager/media_manager.py Outdated Show resolved Hide resolved
langfuse/_task_manager/media_manager.py Outdated Show resolved Hide resolved
langfuse/_task_manager/media_manager.py Outdated Show resolved Hide resolved
langfuse/_task_manager/media_manager.py Outdated Show resolved Hide resolved
langfuse/api/tests/utils/test_http_client.py Outdated Show resolved Hide resolved
langfuse/openai.py Outdated Show resolved Hide resolved
langfuse/openai.py Outdated Show resolved Hide resolved
tests/test_langchain.py Outdated Show resolved Hide resolved
tests/test_langchain.py Show resolved Hide resolved
@hassiebp hassiebp marked this pull request as ready for review November 14, 2024 14:40
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the latest changes and previous review, here is a focused summary of the new updates:

Added robust media handling capabilities with support for multiple content types and reliable background processing.

  • Added support for audio files (MP3, WAV, MPEG) and documents (PDF, text) in /langfuse/api/resources/media/types/media_content_type.py
  • Implemented exponential backoff retry logic in MediaManager for resilient media uploads
  • Added proper cleanup and shutdown handling in TaskManager with atexit registration
  • Added queue size limits (100k items) and batch size constraints in IngestionConsumer to prevent memory issues

The changes build on the previous review by adding more content type support and improving reliability, while maintaining separation between ingestion and media upload concerns.

55 file(s) reviewed, 17 comment(s)
Edit PR Review Bot Settings | Greptile

langfuse/_task_manager/ingestion_consumer.py Show resolved Hide resolved
langfuse/_task_manager/task_manager.py Show resolved Hide resolved
langfuse/_task_manager/task_manager.py Show resolved Hide resolved
langfuse/_task_manager/task_manager.py Outdated Show resolved Hide resolved
langfuse/_task_manager/task_manager.py Show resolved Hide resolved
langfuse/media.py Outdated Show resolved Hide resolved
langfuse/serializer.py Outdated Show resolved Hide resolved
tests/test_decorators.py Outdated Show resolved Hide resolved
tests/test_media.py Show resolved Hide resolved
tests/test_media.py Show resolved Hide resolved
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the latest changes, here is a focused summary of the new updates:

Improved media handling with enhanced error handling and input validation in the Python SDK.

  • Added comprehensive error handling in MediaManager._process_media() with detailed logging and status tracking
  • Implemented input validation for media content length and type in MediaManager._find_and_process_media()
  • Added recursive media object detection in nested data structures with cycle detection using seen set
  • Added proper cleanup of media content bytes after successful upload to prevent memory leaks

The changes focus on making media handling more robust and reliable while preventing potential memory and resource issues.

2 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile

langfuse/decorators/langfuse_decorator.py Outdated Show resolved Hide resolved
langfuse/decorators/langfuse_decorator.py Outdated Show resolved Hide resolved
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the latest changes, here is a focused summary of the new updates:

Enhanced media handling with improved type safety and testing coverage in the Python SDK.

  • Updated LangfuseMedia constructor to use keyword-only arguments for better clarity and type safety
  • Added support for file path-based media loading in LangfuseMedia with proper error handling and logging
  • Added comprehensive test coverage for media handling in test_decorators.py including async generators and streaming
  • Added proper content type validation against MediaContentType enum in media handling

The changes focus on improving the developer experience and reliability of the media handling functionality through better type safety and test coverage.

3 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile

langfuse/_task_manager/task_manager.py Outdated Show resolved Hide resolved
langfuse/media.py Show resolved Hide resolved
langfuse/media.py Outdated Show resolved Hide resolved
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the latest changes, here is a focused summary of the new updates:

Improved thread safety and shutdown handling in task management for the Python SDK.

  • Fixed potential race condition in TaskManager.join() by sequentially pausing and joining ingestion consumers before media upload consumers
  • Added proper timeout handling in MediaManager._process_media() with 1-second queue timeouts for media upload scheduling
  • Added debug logging in TaskManager for better visibility into queue flushing and thread joining operations
  • Implemented exponential backoff with configurable max retries in MediaManager._request_with_backoff()

The changes focus on improving the reliability and observability of the task management system, particularly around graceful shutdown and error handling.

2 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

langfuse/_task_manager/media_manager.py Show resolved Hide resolved
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the latest changes, here is a focused summary of the new updates:

Enhanced ingestion consumer with improved event size management and media handling in the Python SDK.

  • Added intelligent event size management in IngestionConsumer with MAX_EVENT_SIZE_BYTES (1MB) and MAX_BATCH_SIZE_BYTES (2.5MB) limits
  • Added media processing integration in IngestionConsumer._next() using MediaManager.process_media_in_event()
  • Updated CI workflow in .github/workflows/ci.yml to support multimodal testing with docker-compose.v3beta
  • Added package-wide logging configuration in client.py for consistent logging behavior across modules

The changes focus on making event ingestion more robust while properly integrating the new media handling capabilities.

5 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile

langfuse/_task_manager/ingestion_consumer.py Show resolved Hide resolved
tests/test_logger.py Show resolved Hide resolved
tests/test_logger.py Show resolved Hide resolved
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the latest changes, here is a focused summary of the new updates:

Updated CI workflow and Docker configuration for multimodal testing in the Python SDK.

  • Updated Docker Compose file reference from docker-compose.v3beta.yml to docker-compose.v3preview.yml in CI workflow for testing stability
  • Added health check step in CI workflow to verify langfuse server readiness before running tests
  • Added proper logging configuration for server startup and health check in CI workflow
  • Added retry mechanism for server health checks with max 5 attempts and proper error logging

The changes focus on ensuring reliable testing infrastructure for the new multimodal capabilities.

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the latest changes, here is a focused summary of the new updates:

Simplified media upload queue implementation and improved task management in the Python SDK.

  • Replaced custom MediaUploadQueue with standard Queue in media_upload_queue.py for simpler, more maintainable code
  • Added proper timeout handling in MediaManager._process_media() with 1-second queue timeouts for media upload scheduling
  • Added sequential thread shutdown in TaskManager.join() by pausing all consumers before joining to prevent flush interval delays
  • Added proper debug logging in TaskManager.flush() for both ingestion and media upload queue sizes

The changes focus on simplifying the implementation while maintaining reliability and observability of the media upload system.

3 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the latest changes, here is a focused summary of the new updates:

Added comprehensive test coverage for task management and media handling in the Python SDK.

  • Added detailed test cases in test_task_manager.py for truncating large events with _truncate_item_in_place() method
  • Added test coverage for field-specific truncation logic that preserves smaller fields while dropping larger ones
  • Added test cases for handling complex nested JSON structures in event truncation
  • Added test coverage for proper task queue cleanup and thread joining during shutdown

The changes focus on ensuring reliable event size management and proper cleanup behavior through comprehensive test coverage.

Note: This summary focuses on the most recent changes in test_task_manager.py that weren't covered in previous reviews, particularly around event truncation testing and shutdown behavior verification.

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the latest changes, here is a focused summary of the new updates to the Python SDK's multimodal support:

Added robust media handling and event size management in the ingestion consumer and media manager.

  • Added intelligent event truncation in ingestion_consumer.py that preserves smaller fields while dropping larger ones based on size limits (1MB per event, 2.5MB per batch)
  • Added proper media processing in media_manager.py with recursive object detection (max 10 levels deep) and cycle prevention using a seen set
  • Added 1-second timeouts and exponential backoff retries in MediaManager for resilient media upload scheduling and error handling
  • Added comprehensive test coverage in test_task_manager.py for verifying event truncation behavior and proper thread cleanup

The changes focus on making media handling more robust while preventing oversized events from causing API failures.

2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the latest changes, here is a focused summary of the new updates:

Increased health check retries in CI workflow to improve test reliability for multimodal support.

  • Updated max retries from 5 to 10 in CI health check to ensure Langfuse server is ready before running multimodal tests
  • Added proper error logging with docker logs output if health check fails after max retries
  • Added 5-second sleep between retry attempts to give server adequate startup time
  • Added clear success/failure messaging for health check status in CI logs

The changes focus on ensuring the test infrastructure can reliably handle the new multimodal capabilities by giving the server more time to initialize.

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the latest changes, here is a focused summary of the new updates:

Added field-specific media processing and improved error handling in the Python SDK.

  • Added field-specific media processing in MediaManager.process_media_in_event() for "input", "output", and "metadata" fields
  • Added proper trace_id and observation_id extraction from event body with fallback logic for different event types
  • Added proper error handling for missing required fields with ValueError in media processing
  • Added debug logging for media upload queue status and processing in MediaManager

The changes focus on making media handling more robust by properly handling different event types and fields while maintaining clear error reporting.

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the latest changes, here is a focused summary of the new updates:

Added field-specific media processing and improved error handling in the Python SDK.

  • Added proper error handling in MediaManager._process_media() with exponential backoff retries and 1-second queue timeouts
  • Added intelligent event size management in IngestionConsumer with field-specific truncation when exceeding 1MB/2.5MB limits
  • Added health check retries (10 attempts) with 5-second intervals in CI workflow for reliable multimodal testing
  • Added proper trace_id/observation_id extraction with fallback logic in MediaManager.process_media_in_event()

The changes focus on improving reliability and error handling for media processing while ensuring proper test infrastructure support.

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the latest changes, here is a focused summary of the new updates to the Python SDK's multimodal support:

Updated CI configuration and added field-specific media processing improvements.

  • Updated CI workflow to use CLICKHOUSE_MIGRATION_URL instead of LANGFUSE_AUTO_POSTGRES_MIGRATION_DISABLED for proper database setup
  • Added multimodal field processing in MediaManager for "input", "output", and "metadata" with proper trace/observation ID extraction
  • Added proper error handling in MediaManager._process_media() with exponential backoff retries and queue timeouts
  • Added intelligent event size management in IngestionConsumer with field-specific truncation for oversized events

The changes focus on improving CI reliability and enhancing media processing robustness while maintaining proper size constraints.

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the latest changes, here is a focused summary of the new updates to the Python SDK's multimodal support:

Added comprehensive media handling with improved error handling and size management.

  • Added proper error handling for 4xx API errors in MediaManager._request_with_backoff() to fail fast on client errors except rate limits
  • Added proper cleanup of media content bytes in MediaManager._process_upload_media_job() after successful upload
  • Added upload timing metrics with upload_time_ms tracking in media upload status updates
  • Added proper content type validation against MediaContentType enum in media upload requests

The changes focus on improving reliability and observability of media uploads while ensuring proper resource cleanup.

1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

langfuse/media.py Show resolved Hide resolved
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the latest changes, here is a focused summary of the new updates to the Python SDK's multimodal support:

Added robust error handling and API error management in media processing.

  • Added specific handling of 4xx vs 5xx errors in MediaManager._request_with_backoff() to fail fast on client errors while retrying server errors
  • Added proper error propagation in MediaManager.process_media_in_event() with detailed logging of media processing failures
  • Added proper error handling for missing required fields with ValueError in media processing
  • Added proper error handling for API errors in MediaManager._process_upload_media_job() with status code and error text capture

The changes focus on improving error handling robustness and providing better error visibility for media processing operations.

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

@hassiebp hassiebp merged commit 8de2226 into main Nov 16, 2024
11 checks passed
@hassiebp hassiebp deleted the add-multimodal-support branch November 16, 2024 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants